fix(zmodel): change decimal.js to dev dependency for "@zenstackhq/schema" by ymc9 · Pull Request #667 · zenstackhq/zenstack-v3

ymc9 · 2026-02-07T12:34:29Z

Summary by CodeRabbit

Chores
- Reclassified decimal.js as a development-only dependency.

…ema"

claude · 2026-02-07T12:34:42Z

Claude finished @ymc9's task —— View job

PR Review: Move decimal.js to devDependency ✅

I've reviewed this pull request that moves decimal.js from a regular dependency to a dev dependency in the @zenstackhq/schema package.

Todo List:

Read the changed package.json file to understand the dependency modification
Examine the pnpm-lock.yaml changes for consistency
Search codebase for decimal.js usage to assess runtime impact
Check if this change could break functionality
Provide comprehensive review feedback

Review Feedback

✅ Code Quality & Best Practices

This change correctly follows dependency management best practices:

Type-only imports belong in devDependencies
The schema package only uses import type Decimal from 'decimal.js' (packages/schema/src/schema.ts:1)
Decimal is only used for type annotations: MappedBuiltinType = string | boolean | number | bigint | Decimal | Date (line 105)

✅ No Runtime Impact

Safe change: No runtime code in @zenstackhq/schema actually imports or uses decimal.js
Runtime dependencies preserved: Packages that need decimal.js at runtime (@zenstackhq/orm, @zenstackhq/server) already have their own dependencies on it
Proper separation: Schema package provides type definitions; consuming packages handle runtime dependencies

✅ Package Structure

pnpm-lock.yaml updates: Correctly reflects the dependency move without breaking the dependency graph
Workspace consistency: Other packages maintain their decimal.js dependencies as needed

✅ Performance Considerations

Smaller bundle: Removes unnecessary runtime dependency from schema package
Better tree-shaking: Type-only dependencies don't affect bundle size for consumers

✅ Security & Test Coverage

No security concerns: Moving a type dependency to devDependencies doesn't introduce security risks
Test coverage preserved: Runtime functionality remains in packages with proper dependencies

Summary

This is a well-executed optimization that properly categorizes decimal.js as a development-only dependency in the schema package. The change aligns with TypeScript best practices and reduces unnecessary runtime dependencies without breaking functionality.

Recommendation: ✅ Approve - This change improves package dependency hygiene without any functional impact.

coderabbitai · 2026-02-07T12:34:45Z

📝 Walkthrough

Walkthrough

The decimal.js dependency was moved from the dependencies section to the devDependencies section in the schema package configuration. Surrounding commas were adjusted accordingly to maintain valid JSON structure.

Changes

Cohort / File(s)	Summary
Package Configuration `packages/schema/package.json`	Relocated `decimal.js` from dependencies to devDependencies with comma adjustments.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A hop, a skip, a shift so neat,
Decimal.js finds a dev retreat,
From runtime's arms to builder's care,
Dependencies dance through the file with flair! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: moving decimal.js from dependencies to devDependencies in the `@zenstackhq/schema` package.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/schema-decimal

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR adjusts dependency classification for decimal.js in @zenstackhq/schema, aiming to treat it as a development-only dependency rather than a runtime dependency.

Changes:

Moved decimal.js from dependencies to devDependencies in packages/schema/package.json.
Updated pnpm-lock.yaml importer entry for packages/schema accordingly.
Lockfile snapshot entries were also updated as part of the regeneration.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`packages/schema/package.json`	Moves `decimal.js` to `devDependencies` for `@zenstackhq/schema`.
`pnpm-lock.yaml`	Updates lockfile to reflect the dependency move (plus additional snapshot churn).

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-07T12:37:08Z

packages/schema/package.json

    "devDependencies": {
        "@zenstackhq/eslint-config": "workspace:*",
-        "@zenstackhq/typescript-config": "workspace:*"
+        "@zenstackhq/typescript-config": "workspace:*",
+        "decimal.js": "catalog:"
    }


@zenstackhq/schema's public type surface references Decimal from decimal.js (see src/schema.ts exporting MappedBuiltinType with Decimal). Moving decimal.js to devDependencies means consumers installing this package won't get decimal.js, and TypeScript will fail to resolve the generated .d.ts types under pnpm's strict node_modules layout. Keep decimal.js in dependencies (or make it a peerDependency and adjust the exported types to avoid hard-requiring the module).

Copilot · 2026-02-07T12:37:09Z

pnpm-lock.yaml

  eslint-config-next@16.0.1(@typescript-eslint/parser@8.46.2(eslint@9.29.0(jiti@2.6.1))(typescript@5.9.3))(eslint@9.29.0(jiti@2.6.1))(typescript@5.9.3):
    dependencies:
      '@next/eslint-plugin-next': 16.0.1
      eslint: 9.29.0(jiti@2.6.1)
      eslint-import-resolver-node: 0.3.9
      eslint-import-resolver-typescript: 3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.46.2(eslint@9.29.0(jiti@2.6.1))(typescript@5.9.3))(eslint@9.29.0(jiti@2.6.1)))(eslint@9.29.0(jiti@2.6.1))
-      eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.46.2(eslint@9.29.0(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1)(eslint@9.29.0(jiti@2.6.1))
+      eslint-plugin-import: 2.32.0(@typescript-eslint/parser@8.46.2(eslint@9.29.0(jiti@2.6.1))(typescript@5.9.3))(eslint-import-resolver-typescript@3.10.1(eslint-plugin-import@2.32.0(@typescript-eslint/parser@8.46.2(eslint@9.29.0(jiti@2.6.1))(typescript@5.9.3))(eslint@9.29.0(jiti@2.6.1)))(eslint@9.29.0(jiti@2.6.1)))(eslint@9.29.0(jiti@2.6.1))
      eslint-plugin-jsx-a11y: 6.10.2(eslint@9.29.0(jiti@2.6.1))


The lockfile includes changes to eslint-plugin-import/resolver snapshot entries in addition to the intended decimal.js dependency move. If possible, regenerate pnpm-lock.yaml with the repo-pinned pnpm@10.23.0 and ensure only dependency graph changes related to the decimal.js move are included to reduce review/merge noise.

fix(zmodel): change decimal.js to dev dependency for "@zenstackhq/sch…

0955a20

…ema"

Copilot AI review requested due to automatic review settings February 7, 2026 12:34

Copilot started reviewing on behalf of ymc9 February 7, 2026 12:34 View session

Copilot AI reviewed Feb 7, 2026

View reviewed changes

ymc9 closed this Feb 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(zmodel): change decimal.js to dev dependency for "@zenstackhq/schema"#667

fix(zmodel): change decimal.js to dev dependency for "@zenstackhq/schema"#667
ymc9 wants to merge 1 commit intodevfrom
fix/schema-decimal

ymc9 commented Feb 7, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

claude bot commented Feb 7, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Feb 7, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

Copilot AI Feb 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ymc9 commented Feb 7, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

claude bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Move decimal.js to devDependency ✅

Todo List:

Review Feedback

✅ Code Quality & Best Practices

✅ No Runtime Impact

✅ Package Structure

✅ Performance Considerations

✅ Security & Test Coverage

Summary

Uh oh!

coderabbitai bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ymc9 commented Feb 7, 2026 •

edited by coderabbitai bot

Loading

claude bot commented Feb 7, 2026 •

edited

Loading

coderabbitai bot commented Feb 7, 2026 •

edited

Loading